Objects treated as missing despite being present, due to race with geometric repacking - #2207

Changes since v2:

• Ripped out the old 3/4 dealing with QUICK readers; QUICK readers are left alone
• Insert a new 3/4 fixing git mktree --batch to stop passing QUICK (with new testcase in t1010)
• undo bad paragraph comment change
• renamed fill_midx_entry() -> midx_fill_entry(), so that we catch any other new callers and appropriately check their return value (caught one in test-read-midx.c)

Changes since v1:

• Rebased on top of ps/odb-generic-corrupt-objects, and conflicts with it resolved
• Removed useless test_grep line spotted by Junio in PATCH 1
• Switched fill_midx_entry() to a tri-state to avoid duplicate bsearch_midx(), as suggested by Peff
• Only do the re-read on SECOND_READ, as suggested by Peff
• Handle multiple objects shared across multiple packs correctly (issue caught & corrected & new testcase by deeper AI review)
• Inserted two new patches:
  • 2/4: Fix a leak in git mktree --batch since I use it in new testcases and don't want the *-leaks jobs failing
  • 3/4: Demonstrate and fix QUICK reader problems, while keeping expected QUICK performance for normal cases (we've already been discussing this patch in this thread a bunch anyway, and it's logically related)

Cover letter addendum/update:

We also fix git mktree --batch to no longer erroneously pass QUICK.

Note here that Stolee's suggestion to defer pack deletion via git multi-pack-index expire seems like a good complementary mitigation; it would reduce how often we fall into recovery, while this series tries to fix recovery to work more robustly.

Original cover letter (focused on the final patch):

When an object is found in multiple packs that are in a multi-pack-index, and a subsequent geometric repacking creates a new multi-pack-index and removes the pack that was considered the owner of the object in the old multi-pack-index, then an already-running process that had opened the old multi-pack-index and hadn't yet opened the removed packfile will not be able to access the object -- lookups will return it as missing. Additionally, replay has a separate bug where a missing object causes a SIGSEGV rather than an error message.

This appears to affect a very small percentage of git operations in production since it is a tiny window, but I've found evidence of it occurring in at least eight distinct server-side operations, covering seven different git commands:

git operation                        symptom
-----------------------------------  -----------------------------
git replay (server-side rebase)      SIGSEGV (this series, 1/2)
git merge-tree                       spurious read-miss failure
git diff (raw and tree-vs-tree)      spurious read-miss failure
git rev-list --count                 spurious read-miss failure
git merge-base                       spurious read-miss failure
object/rev resolution (rev-parse,    spurious read-miss failure
  cat-file)
repository repair (fsck/repack)      spurious read-miss failure

There are also commands that could be changing behavior without throwing an error -- e.g. object negotiation thinking an object doesn't exist and instead negotiating based on an older common commit, or cat-file --batch reporting that some objects don't exist.

This series fixes the replay bug first, since it's simpler; investigating it, together with my other recent repacking work, is what led me to the underlying multi-pack-index issue that 2/2 addresses.

cc: Patrick Steinhardt ps@pks.im
cc: Elijah Newren newren@gmail.com
cc: Jeff King peff@peff.net
cc: Derrick Stolee stolee@gmail.com